Skip to content

Conversation

jycouet
Copy link
Contributor

@jycouet jycouet commented Sep 18, 2025

No description provided.

Copy link

changeset-bot bot commented Sep 18, 2025

🦋 Changeset detected

Latest commit: b3755c6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sv Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Sep 18, 2025

Open in StackBlitz

npx https://pkg.pr.new/sveltejs/cli/sv@719
npx https://pkg.pr.new/sveltejs/cli/svelte-migrate@719

commit: b3755c6

Copy link
Member

@AdrianGonz97 AdrianGonz97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great! for some reason, i'm unable to push some changes to this pr, so i've added some suggestions below.

Comment on lines +21 to +25
".": "./index.ts",
"./css": "./tooling/css/index.ts",
"./html": "./tooling/html/index.ts",
"./js": "./tooling/js/index.ts",
"./parsers": "./tooling/parsers.ts"
Copy link
Member

@AdrianGonz97 AdrianGonz97 Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll have to keep in mind that @sveltejs/cli-core will be a public package in the future, so this will have to be updated back to its original state when community add-ons are released

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it be @sveltejs/cli-core or maybe under sv like sv/core ?
What about @sveltejs/create & @sveltejs/addons ?

It might be a question for later anyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it be @sveltejs/cli-core or maybe under sv like sv/core ?

its still TBD, but our plans were to have core be its own standalone package so that community add-ons don't have sv as a direct dependency

What about @sveltejs/create & @sveltejs/addons ?

create should probably stay in sv, but there's an argument to be made as to where addons should go. it may be more useful to have it be part of core but we can figure it out when we get there :p

@mrginglymus
Copy link

mrginglymus commented Sep 19, 2025

Thanks, types are coming through now!

There are a few types that could be usefully reexported (TemplateType and LanguageType) to avoid type TemplateType = Parameters<typeof create>[1]["template"];

What's significantly missing though is typed official addons - because they're exported as a list, there's no way have type-safe configured addons:

export const officialAddons = [

Could we change this to

export const officialAddons = {
  prettier,
  eslint,
  ...,
} satisfies Record<string, AddonWithoutExplicitArgs> as const;

or similar? That way a user will be able to define, in a type safe way, a set of preconfigured options for the addons.

@jycouet
Copy link
Contributor Author

jycouet commented Sep 19, 2025

I should not have created the PR! that's why we can't push there. :/ (TIL!)
Now @sxzz invited me to his repo, I'll try to apply suggestions (today) and see 👍

EDIT: it seems that you can push now @AdrianGonz97

@jycouet
Copy link
Contributor Author

jycouet commented Sep 19, 2025

Could we change this to

export const officialAddons = {
  prettier,
  eslint,
  ...,
} satisfies Record<string, AddonWithoutExplicitArgs> as const;

or similar? That way a user will be able to define, in a type safe way, a set of preconfigured options for the addons.

Before exporting this, we need to fix pnpm tsc. (Similar I think)
I'm not sure where it's coming from as the typing was good before (internally). The thing is that they are not all AddonWithoutExplicitArgs.

Who wants to crack the matrix ? :D

@mrginglymus
Copy link

Well I thought I'd fixed pnpm check, but....rolldown doesn't support custom export conditions?

@sxzz
Copy link
Contributor

sxzz commented Sep 19, 2025

Of course, Rolldown supports it.

@mrginglymus
Copy link

mrginglymus commented Sep 19, 2025

Something in my current toolchain (via rollup-plugin-dts) is dropping it...

@manuel3108 manuel3108 marked this pull request as draft September 24, 2025 16:12
@manuel3108
Copy link
Member

Converted to draft, dosn't seem ready yet according to the comments

@AdrianGonz97
Copy link
Member

AdrianGonz97 commented Sep 25, 2025

exposing the officialAddons map is turning into a bit of a head scratcher with relation to tsdown.

atm, i'm trying to disable oxc (as well as disabling isolatedDeclarations) and use tsc instead to generate the types through tsdown, but it seems that rolldown-plugin-dts is getting hung up on some of the types that originate from @sveltejs/create, and are reexported from the cli:

[plugin rolldown-plugin-dts:fake-js]
SyntaxError: Export 'LanguageType' is not defined. (57:35)

i have a feeling this may be a rolldown-plugin-dts bug, but im unsure. have any ideas @sxzz?

@AdrianGonz97
Copy link
Member

i guess we could keep using isolatedDeclarations with oxc, but it would force us to generate types that aren't really ideal:

img

with Addon<any>, we lose the types of the addon's options, but keep the typed keys for each entry (e.g. officialAddons.prettier, etc). we'd also have to maintain the OfficialAddons type, but that's a rather small annoyance.

ideally, using as const (shown on the left) would be the best, but it doesn't seem to be compatible with isolatedDeclarations

@AdrianGonz97 AdrianGonz97 marked this pull request as ready for review September 29, 2025 17:36
Copy link
Member

@AdrianGonz97 AdrianGonz97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not ideal, but will do for now just to move things along

@AdrianGonz97 AdrianGonz97 merged commit b48c291 into sveltejs:main Sep 29, 2025
12 of 13 checks passed
@github-actions github-actions bot mentioned this pull request Sep 28, 2025
@sxzz sxzz deleted the feat/tsdown branch September 30, 2025 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants